Skip to content

Fix for lattice energy UnitConv bug#144

Merged
T-Nicholls merged 5 commits intoDiamondLightSource:mainfrom
T-Nicholls:main
Aug 7, 2025
Merged

Fix for lattice energy UnitConv bug#144
T-Nicholls merged 5 commits intoDiamondLightSource:mainfrom
T-Nicholls:main

Conversation

@T-Nicholls
Copy link
Copy Markdown
Member

@T-Nicholls T-Nicholls commented Jul 25, 2025

Also a bonus commit used by Python BBA

@T-Nicholls T-Nicholls requested review from MJGaughran and ptsOSL July 25, 2025 13:44
@T-Nicholls T-Nicholls mentioned this pull request Jul 25, 2025
Merged
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 25, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.11%. Comparing base (c32f154) to head (27571aa).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
- Coverage   99.48%   99.11%   -0.38%     
==========================================
  Files          12       12              
  Lines         775      788      +13     
==========================================
+ Hits          771      781      +10     
- Misses          4        7       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@T-Nicholls
Fix CI failures
3a39b20 
- Switch to (up/down)load-artifact@v4 as v3 is now fully deprecated
- Ensure all of the build artifacts have unique names as v4 doesn't support multiple artifacts with the same name
- Drop Python 3.7 from the CI matrix as v4 doesn't support it
- Fix a couple of docstrings to resolve linting check failure
@T-Nicholls
Copy link
Copy Markdown
Member Author

T-Nicholls commented Jul 25, 2025

Ok, CI failures are resolved and I've cleaned up the commit history so this is ready to merge

However, we should get Pytac properly on the latest Copier template at some point

ptsOSL
ptsOSL reviewed Jul 28, 2025
View reviewed changes
Comment thread src/pytac/load_csv.py Outdated
ptsOSL
ptsOSL reviewed Jul 28, 2025
View reviewed changes
Comment thread src/pytac/load_csv.py
ptsOSL
ptsOSL reviewed Jul 28, 2025
View reviewed changes
Comment thread src/pytac/data/utils/load_unitconv.m
@ptsOSL
Copy link
Copy Markdown
Collaborator

ptsOSL commented Jul 28, 2025

I agree that we need to convert this to copier template, probably a job for me alongside converting slow feedbacks.

MJGaughran
MJGaughran approved these changes Aug 6, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@MJGaughran MJGaughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you've both highlighted several existing problems in pytac (e.g. with type hinting), but I agree that this PR is not making things worse than before.

Otherwise, it all looks fine.

@T-Nicholls T-Nicholls merged commit a6256df into DiamondLightSource:main Aug 7, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MJGaughran MJGaughran MJGaughran approved these changes

@ptsOSL ptsOSL ptsOSL approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@T-Nicholls @ptsOSL @MJGaughran